feat: Add SwapToast component #1281
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
abcrane123
force-pushed
the
alissa.crane/swap-toast
branch
from
358b456
to
04e3576
Compare
| @@ -70,6 +70,9 @@ export function SwapProvider({ | |||
| }, | |||
| }); // Component lifecycle | |||
|
|
|||
| const [isToastVisible, setIsToastVisible] = useState(false); | |||
| const [transactionHash, setTransactionHash] = useState(''); | |||
There was a problem hiding this comment.
thoughts on combining these to store toast content in isToastVisible? would allow the us to use the toast for other any information instead of being strictly tied to success/transactionHash
i.e. const [toastContent, setToastContent] = useState<bool | ReactNode>(false);
setToastContent(<SuccessToast transactionHash=""/>);
There was a problem hiding this comment.
hmm well we don't really need to store the whole component because theyre rendering SwapToast as a child (following the pattern of the rest of the components) right? also we could use the toast for other information by checking the isToastVisible value along with an error state, for example, stored in lifeCycleStatus. The reason i had to do the extra state for transactionHash is because we immediately reset the lifeCycleStatus to init after calling onsuccess.
curious what the rest of the team thinks about this @0xAlec @cpcramer
my thinking is that if we want to do something like you're suggesting we should follow the same pattern with TransactionToast and maybe that should be a follow up.
There was a problem hiding this comment.
I think it makes sense regardless to not have the toast showing content directly from lifecycle. If we want to toast an error, we don't want the toast to clear if the error clears, but I don't want to end up with a bunch of separate states to hold different things we want to show in the toast. my initial thought was that success toast could store transactionHash in its internal state and encapsulate the chainExplorer logic, and then just get displayed as a child of the toast.
| background.default, | ||
| 'flex animate-enter items-center justify-between rounded-lg', | ||
| 'p-2 shadow-[0px_8px_24px_0px_rgba(0,0,0,0.12)]', | ||
| '-translate-x-2/4 fixed z-20', |
There was a problem hiding this comment.
the animate in from the right seems weird, also, should this be pegged to the swap component vs. the window?
There was a problem hiding this comment.
Yeah i noticed that as well but it seems like only an issue with playground. The same issue appears for transaction toast which looks fine on docs site.
There was a problem hiding this comment.
so will investigate but it appears like it was an existing issue with how we style our toasts
| @@ -0,0 +1,12 @@ | |||
| export function getToastPosition(position: string) { | |||
| isToastVisible?: boolean; | ||
| setIsToastVisible?: (visible: boolean) => void; |
There was a problem hiding this comment.
I think a larger discussion, but how are we determining when state should live / be handled in the SwapContext vs. LifecycleStatus shared? cc @alessey
There was a problem hiding this comment.
I think the toasts are a case where we cannot rely on lifecycle status in case we want to display a toast longer than a lifecycle status persists. Although I would prefer to encapsulate the toast state within its own component or a single toast state.
What changed? Why?
SwapToastcomponent and display on successNotes to reviewers
How has it been tested?